Skip to content

Add gix blame -L start,end #1766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jan 16, 2025
Merged

Add gix blame -L start,end #1766

merged 3 commits into from
Jan 16, 2025

Conversation

@cruessler
Copy link
Contributor

@cruessler cruessler commented Jan 13, 2025

This enables running blame for a portion of a file. This feature will be helful for debugging gix blame, in particular in the context of #1743.

Byron
Byron requested changes Jan 14, 2025
View reviewed changes
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for making blame even better!

Besides my comments which are more like nits to let it appear less rushed, one change I really have to ask for: Changes to gix-blame need to go into their own commit, and be prefixed with feat!: to indicate a breaking change. A single test there probably would be a good idea (or necessity) as well.

The changes to the other two crates can be bundled as they logically are one (core + gitoxide).

Thanks a lot!

@cruessler
Copy link
Contributor Author

cruessler commented Jan 14, 2025

Thanks a lot for making blame even better!

Besides my comments which are more like nits to let it appear less rushed, one change I really have to ask for: Changes to gix-blame need to go into their own commit, and be prefixed with feat!: to indicate a breaking change. A single test there probably would be a good idea (or necessity) as well.

The changes to the other two crates can be bundled as they logically are one (core + gitoxide).

Thanks a lot!

Sure, no worries, and thank for letting me know! I’m not fully familiar with all the conventions yet, so always grateful to learn them. :-)

@cruessler cruessler force-pushed the add-range-to-gix-blame branch from f99eacf to 1f67966 Compare January 14, 2025 14:56
cruessler and others added 2 commits January 16, 2025 08:00
@cruessler @Byron
feat: add gix blame -L start,end
4a78395
@Byron
refactor
1500c08 
- don't use closures if they con't enclose state (or don't need to)
@Byron Byron force-pushed the add-range-to-gix-blame branch from 1f67966 to 1500c08 Compare January 16, 2025 07:01
@Byron
Copy link
Member

Byron commented Jan 16, 2025

Thanks a lot for the fixes!

Additionally, I have modified the second commits subject to not contain a ! as it's not a breaking change, just a new feature.

@Byron Byron enabled auto-merge January 16, 2025 07:04
@Byron Byron merged commit 90fef01 into GitoxideLabs:main Jan 16, 2025
19 checks passed
@cruessler cruessler deleted the add-range-to-gix-blame branch January 16, 2025 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Byron Byron Byron approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cruessler @Byron